Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change how HBMenu replacement is done and some small fixes #41

Closed
wants to merge 4 commits into from

Conversation

shadow2560
Copy link
Contributor

I've tried to apply my idea explained in issue #34, it's probably not the cleaner way to do it but at least it works as I wanted and you can see more precisely what is my idea. I've also fixed some small bugs and updated the french translation to add some missing strings.

…mebrews as stared, update french translation to include some missing strings.

Signed-off-by: shadow2560 <[email protected]>
… /hbmenu.nro, if not it copy the version launched to /switch/sphaira/sphaira.nro then delete the launched nro and finaly launch the copied file. This is needed for the app update process witch copy the file downloaded with the path in the zip witch is for now /switch/sphaira/sphaira.nro and not the path of the app launched and this will simplify some possible functions added in the futur.

Signed-off-by: shadow2560 <[email protected]>
…aira/sphaira.nro during update, prevent possible problems if a zip update is differently formed.

Signed-off-by: shadow2560 <[email protected]>
@ITotalJustice
Copy link
Owner

thanks for the pr, i now understand what you wanted in #34

the replace hbmenu on exit option design was intentionally a config option as it is necessary to always replace hbmenu on exit (if thats what the user wants). this option was inspired by hekate replacing reboot_payload.bin if it detects that it is not hekate.

lets say a user launches sphaira as normal, likes it and decides they want to use it in place of hbmenu, they see the option to do that an enable it. later down the line, they update atmosphere which replaces /hbmenu.nro (which is sphaira) with hbmenu . all the user needs to do is launch sphaira (which will exist in /switch/sphaira/sphaira.nro unless they manually deleted it) and it will replace hbmenu for them, as that option is saved in the config. without this, the user must enable this option everytime they update atmosphere, which i am not a fan of. i'd rather them just have to launch sphaira and be done with it.


i disagree with the the fixing of paths such as moving the nro to a new location, this could break forwarders. it also breaks my use case as i nxlink sphaira to /switch/sphaira (i know i can specify the path, im just lazy) so this would result in it moving the nro and relaunching, which closes nxlink. overall, i dont think we should put hard limits as to where the user places the nro. /switch/ or /switch/sphaira is fine, its why i handle both cases throughout my code (aside from the updater, but i'll fix that).


i feel like the main flaw with my version with replacing the hbmenu with sphaira is that i offered no version to restore hbmenu. your version helps to address that - even if the user manually replaces it themselves. i figured the easiest fix would be to show a pop up asking if they want to restore hbmenu if they disable the replace option. if the option is already disabled because the user manually replaced hbmenu, then it's safe to assume that they really wanted to replace hbemnu, and if they're capable of manually replacing hbmenu, then they're capable of manually replacing sphaira should they want to switch back.

@shadow2560
Copy link
Contributor Author

I understand and I have not thinked of these cases (especialy for Nxlink), I'm agree. So I'm closing this pr. I let you fix the small bugs that I have also fixed in this pr (logging stared homebrew and missing strings in french language), I'll also close the issue #34.
PS: For the updater my aproach in the last commit of this pr could be a good starting point, just need to adapt a little finaly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants